- 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.7k
 
Update Karpenter to v1.6.2 #17567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Karpenter to v1.6.2 #17567
Conversation
| 
           /override pull-kops-e2e-aws-upgrade-k129-ko129-to-k130-kolatest-karpenter  | 
    
| 
           @hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-aws-upgrade-k129-ko129-to-k130-kolatest-karpenter In response to this: 
 Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.  | 
    
| 
           FYI I started working on a model that translates an InstanceGroup to EC2NodeClass and NodePool resources: master...rifelpet:kops:karpenter-model I was imagining that we'd add a new karpenter controller to kops-controller that periodically fetches the instance groups (and this new userdata object) from VFS, generates the model, and reconciles the resulting resources. It should be straight forward to add models for other cloud providers too.  | 
    
          
 Very nice @rifelpet, I think that is exactly the missing piece. This PR adds generates a file with the user data, which is not that easy to generate separately. This way, everything needed for generating the   | 
    
096f26e    to
    7343e7b      
    Compare
  
    c8e16df    to
    5132f34      
    Compare
  
    | 
           /test pull-kops-verify-terraform  | 
    
| 
           /override pull-kops-e2e-aws-karpenter  | 
    
| 
           @hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-aws-karpenter In response to this: 
 Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.  | 
    
| 
           /test pull-kops-scenario-aws-karpenter  | 
    
93575b2    to
    b84c2a4      
    Compare
  
    | 
           /test pull-kops-scenario-aws-karpenter  | 
    
b84c2a4    to
    1433cd1      
    Compare
  
    1433cd1    to
    3deb6b8      
    Compare
  
    | 
           /test pull-kops-scenario-aws-karpenter  | 
    
| 
           /retest  | 
    
          
 @ameukam I think we decided to merge and wait for Peter's PR another 3 weeks.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we mention in the docs that upgrading a cluster from the old version isn't supported. This is probably worthy of a release note, especially if someone has been able to have a usable kops 1.33 cluster with karpenter 0.31.3.
| nth := clusterSpec.CloudProvider.AWS.NodeTerminationHandler | ||
| if nth.Enabled == nil { | ||
| if clusterSpec.Karpenter != nil && clusterSpec.Karpenter.Enabled { | ||
| nth.Enabled = fi.PtrTo(false) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we check this in api validation? to ensure spec.karpenter.enabled != spec.cloudprovider.aws.nodeTerminationHandler.enabled ?
| app.kubernetes.io/managed-by: Helm | ||
| spec: | ||
| replicas: {{ ControlPlaneControllerReplicas false }} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we preserve this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, but added back for now.
| {{ if not IsIPv6Only }} | ||
| dnsPolicy: Default | ||
| {{ else }} | ||
| # Must use ClusterFirst on IPv6 clusters in order to get DNS64 | ||
| dnsPolicy: ClusterFirst | ||
| {{ end }} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and preserve this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good idea, added back.
Signed-off-by: Ciprian Hacman <[email protected]>
Signed-off-by: Ciprian Hacman <[email protected]>
          
 Thanks for the review. Added the release notes piece along with the dns and replicas, so should be ready to go now.  | 
    
| 
           /test pull-kops-scenario-aws-karpenter  | 
    
Signed-off-by: Ciprian Hacman <[email protected]>
f1f9e85    to
    efdc249      
    Compare
  
    | 
           /test pull-kops-scenario-aws-karpenter  | 
    
| 
           /override pull-kops-kubernetes-e2e-ubuntu-gce-build  | 
    
| 
           @hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-aws-upgrade-k133-ko133-to-klatest-kolatest-many-addons, pull-kops-kubernetes-e2e-ubuntu-gce-build In response to this: 
 Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.  | 
    
9d4ea86    to
    efdc249      
    Compare
  
    | 
           /test pull-kops-scenario-aws-karpenter  | 
    
| 
           @hakman: Overrode contexts on behalf of hakman: pull-kops-e2e-aws-upgrade-k133-ko133-to-klatest-kolatest-many-addons, pull-kops-kubernetes-e2e-ubuntu-gce-build In response to this: 
 Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.  | 
    
| 
           [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here 
Needs approval from an approver in each of these files:
 
      
 Approvers can indicate their approval by writing   | 
    
| 
           /unhold  | 
    
Iterating on updated Karpenter setup.
Includes:
kops-controllerto generate theEC2NodeClassLaunchTemplategeneration, which can no longer be used by Karpenter./cc @rifelpet @justinsb @ameukam